This is a small improvement to better reflect actual streaming use cases.

Verification that the labels are being applied properly now.

I am not following, where in this function do we use the reqCtx that would require moving this logic earlier? it is fine to move it up, but wanted to double check where the bug is in this specific case.

should we fix ExtractHeaderValue then? see https://github.com/kubernetes-sigs/gateway-api-inference-extension/blob/main/pkg/epp/util/request/headers.go#L35;

Probably best if we have a function for the if/else logic that we use across the code for reading the header value and name it ExtractHeaderValue and rename the current ExtractHeaderValue to ExtractCaseInsensitiveHeaderValue

Yes, this is a much cleaner solution, thanks! I missed this helper as I was tracing the request path. Most of these were discovered through hermetic test failures post-refactoring, and I am not deeply familiar with this part of the codebase. I will consolidate this into the helper.

trying to understand if this is really a bug, in the case the header already existed, HandleRequestHeaders would have set it in the reqCtx.Request.Headers, right? I guess the only difference here is that we are unifying the header key as requtil.RequestIdHeaderKey since at line 187 the header could have had a different casing.

since at line 187 the header could have had a different casing

Let me look into this more. You may be onto something here. Perhaps there is a simpler solution / root cause for the behaviors I was seeing.

Two comments:

1. From metrics perspective, we are not truly accumulating anything, this is just reporting the values in the last block.

2. Which logic depends reqCtx.Usage while we are accumulating the stream that would require us to keep updating the usage rather than just doing it at the end?

The issue here was that the 'usage' block was not coming in the final "[DONE]" message in the hermetic testing environment. It was arriving in the penultimate message instead.

When processing the final message of the stream, it was empty, resulting in underreporting and empty labels. I need to do some more investigating to understand if this is an issue with the hermetic testing environment or something that can also occur with real traffic.

From metrics perspective, we are not truly accumulating anything, this is just reporting the values in the last block.

True, we just need to find the block that populates it. My comment is misleading as this is still a "detect and write once" process that is only reported after the stream ends.

I was trying to understand and debug behavior from integration test results, and I am not too familiar with our request stream handling code and the properties we can rely upon in production. It's very possible my root cause analysis is wrong in places, but the symptoms I reported in the linked issues are reproducible.

Let me write some targeted unit tests over the relevant components to better understand this.

The issue here was that the 'usage' block was not coming in the final "[DONE]" message in the hermetic testing environment. It was arriving in the penultimate message instead.

This might be an issue in the hermetic test. In real traffic, the penultimate and last message come in a same chunk:

data: {"id":"...","object":"text_completion","created":1739400043,"model":"food-review-0","choices":[], "usage":{"prompt_tokens":7,"total_tokens":17,"completion_tokens":10}}

data: [DONE]

Great, thanks for confirming! I will fix the hermetic test setup to conform to this.

I think #1626 can be closed as WAI. I also reported an issue where metric labels were not being set properly, so I need to first confirm that this is solely triggered by the data: [DONE] final chunk case. If so, I'll revert my change to this response handling code.

continuing @ahg-g line of thinking - I agree we should fix the ExtractHeaderValue helper func and then we can use it here, e.g.,:

reqCtx.Request.Headers[key] = ExtractHeaderValue(key)

I think the logic that handles RawValue or Value should be scoped to a single helper function.

use the single helper function?
RawValue/Value should be encapsulated to ExtractHeaderValue helper func.

ditto

this is great

The primary focus of this PR is a significant refactoring of the EPP hermetic test suite to achieve true test isolation. While it also includes important bug fixes that this new test rigor uncovered, the change here was necessary to unblock upcoming integration tests for the Flow Control layer, which require a clean and mutable environment for each test scenario.

Guidance for Reviewers

I would greatly appreciate your focus on the test refactoring itself:

• Isolation Strategy: Does the combination of unique labels and a scoped cache seem robust and effective for serial execution?
• Clarity & Maintainability: Is the new testHarness approach clearer and easier to extend?
• Potential Risks: Do you see any potential downsides to this per-test setup approach?

I just want to make sure this doesn't get overlooked amidst the bug fix discussion as this is the heart of this PR.

Drawbacks of the Previous Approach

Previously, our tests relied on a shared, suite-level setup. The controller-runtime manager's cache was unscoped, meaning it watched all pods in the namespace. This design was fragile and prone to state leakage between test runs, making it difficult to write reliable tests for features that need to mutate backend state (e.g., marking pods as saturated/unsaturated).

The New testHarness

This PR overhauls this approach by introducing a testHarness that provides a fully encapsulated, hermetic environment for each test case.

Here’s a summary of the key isolation mechanisms:

1. Per-Test Harness: Each test now runs within its own testHarness instance, which encapsulates all necessary components (server, client, manager) and state.
2. Unique Resource Labeling: A unique test-run-id label (derived from a UUID) is generated for each harness and applied to all Kubernetes pods it creates.
3. Scoped Kubernetes Cache: This is the most crucial part. The controller-runtime manager within each harness is configured with a cache that only watches pods matching that specific, unique test-run-id label. This prevents the test from seeing or interacting with pods from any other test run.
4. Dedicated Lifecycle Management: Each harness starts its own server instance and creates its own gRPC client. A robust cleanup function, automatically called by t.Cleanup, uses a context.CancelFunc to gracefully tear down the server, manager, and client, and then deletes the uniquely labeled Kubernetes pods.

A Note on Parallelism

Despite this isolation, tests cannot use t.Parallel() due to controller-runtime's global metrics registry. Our solution for clean serial execution is resetting the registry post-test (t.Cleanup(metrics.Reset)). True parallelism would require per-test envtest instances, which is prohibitively slow. This refactoring is a pragmatic step, solving resource contention while sharing the suite-level envtest setup.

Thanks!